Control showing of notifications on iOS by rajveermalviya · Pull Request #2156 · zulip/zulip-flutter

rajveermalviya · 2026-02-18T16:59:39Z

Testing

iOS Notification Service doesn't execute unless the push notification has the "mutable-content" : 1 property under "aps" object.

Zulip server doesn't set this property in non-e2ee push notifications, so the dev server will need the patch below to do the testing:

diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py
index 4f9f0de5ae..f6da5c12fb 100644
--- a/zerver/lib/push_notifications.py
+++ b/zerver/lib/push_notifications.py
@@ -323,6 +323,7 @@ def send_apple_push_notification(
             num_duplicate_tokens,
         )
     payload_data = dict(payload_data)
+    payload_data["mutable-content"] = 1
     message = {**payload_data.pop("custom", {}), "aps": payload_data}
 
     have_missing_app_id = False

Additionally, the iOS Notification Service doesn't execute with the simulated push notifications as described in "Testing Push Notifications on iOS Simulator" docs. It only executes if the push notification is a real push notification either from the Sandbox/Production APNs server. So, the dev server will need to setup those as documented here.

Fixes: #1265

rajveermalviya · 2026-02-25T17:38:51Z

This is ready for review now!

rajveermalviya · 2026-02-26T19:00:27Z

In the pariring call today, Greg metioned that there are some commits here that could be squashed — I'll do that tommorow. (Namely: swift-format into swift code changes, flutter build ios --config-only into the commits that require them, and some steps from the guide could be merged).

chrisbobbe

Thanks, this is exciting!! Comments below, from reading the commits. I haven't yet done manual testing.

chrisbobbe · 2026-02-27T06:30:53Z

+		<key>NSExtensionPointIdentifier</key>
+		<string>com.apple.usernotifications.service</string>
+		<key>NSExtensionPrincipalClass</key>
+		<string>$(PRODUCT_MODULE_NAME).NotificationService</string>


ios: Add Notification service app extension target via Xcode

Commit-message nit: I think you mean "NotificationService" or "notification service", not "Notification service"? Depending on whether or not you want to refer to the specific name that appears in the code.

chrisbobbe · 2026-02-27T06:35:13Z

+//  NotificationService.swift
+//  NotificationService
+//
+//  Created by Rajesh Malviya on 18/02/26.


chrisbobbe · 2026-02-27T06:41:49Z

 				INFOPLIST_KEY_CFBundleDisplayName = NotificationService;
 				INFOPLIST_KEY_NSHumanReadableCopyright = "";
-				IPHONEOS_DEPLOYMENT_TARGET = 26.2;
+				IPHONEOS_DEPLOYMENT_TARGET = 15.0;


The "iOS Deployment Target" of "Runner" is something we mention in a comment at the top of ios/Podfile:

# This should match the iOS Deployment Target # (in Xcode, that's in project > Runner > Info) # and MinimumOSVersion in ios/Flutter/AppFrameworkInfo.plist. platform :ios, '15.0'

Should that comment grow a mention of this new thing that needs to be set to the same value? I believe this is where we declare the minimum supported iOS version, so it's definitely something we expect to change in the future.

chrisbobbe · 2026-02-27T07:01:45Z

+			);
+			runOnlyForDeploymentPostprocessing = 0;
+			shellPath = /bin/sh;
+			shellScript = "/bin/sh \"$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh\" build\n";


ios: In Xcode, add Run Script build phase for the extension target

Hmm, just above this B378A52C2F45C31A0031EFA1 /* ShellScript */ = { block is a nearly identical block 9740EEB61CF901F6004384FC /* Run Script */ = { which has the same shellScript value. Are they both needed? I wonder if the problem was just that the old way of expressing the desired build phase wasn't working.

The previous 9740EEB61CF901F6004384FC /* Run Script */ block is referenced by the Runner target, and B378A52C2F45C31A0031EFA1 /* ShellScript */ is being added by this workaround/step for the NotificationService target (Not sure why the (commented) name generated here is different, in Xcode both phases, under TARGETS > [Runner/NotificationService] > Build Phases (tab) > Run Script, are called "Run Script".

chrisbobbe · 2026-02-27T07:06:26Z

+// `assert()` statements are not working in the extension even when running in
+// the debug mode. But fortunately `kDebugMode` does correctly return true in
+// the debug mode and false in the release mode. So, use this helper instead of
+// `debugLog` from `log.dart`.


Maybe worth a // TODO debug asserts not working on this.

chrisbobbe · 2026-02-27T07:16:39Z

+    // current thread is being polled by the system. Which is not the case in
+    // Notification Service Extension, so here we manually poll the event loop.
+    // See discussion:
+    //   https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/Running.20Dart.20code.20in.20iOS.20Notification.20Service.20Extension/with/2370721


Will there be a change to be made here if a PR like flutter/flutter#181645 lands? If so, let's add a TODO. (I'm not familiar with all the details but I found that PR by following links.)

chrisbobbe · 2026-02-27T07:21:35Z

+// This library defines the Dart entrypoint function for headless FlutterEngine
+// used in iOS Notification Service Extension. We need to import it here to
+// avoid it being treeshaked during build process.
+// ignore: unused_import
+import 'notifications/ios_service.dart';


I wonder if this is the standard way to prevent something from being treeshaken, or if there's some config in the build process for this?

I believe @pragma('vm:entry-point') decorator prevents the top-level function from being tree-shaked but AIUI there's no way to prevent the whole unimported dart library from being tree-shaked. (I realize tree-shaked is not the correct term, it's more that the dart library is not imported anywhere so it's not included in the build.)

chrisbobbe · 2026-02-27T07:24:14Z

+    guard let bestAttemptContent = bestAttemptContent else {
+      return
+    }


What is this early return doing? I'm not sure if this needs a comment or if I just need to learn more Swift. 🙂

chrisbobbe · 2026-02-27T07:27:04Z

+      switch result {
+      case .success(let mutatedNotificationContent):
+        os_log("didReceivePushNotification: success")
+        bestAttemptContent.title = mutatedNotificationContent.title
+        if let body = mutatedNotificationContent.body {
+          bestAttemptContent.body = body
+        }
+        contentHandler(bestAttemptContent)
+
+      case .failure(let error):
+        os_log("didReceivePushNotification: failed: error=\(error.localizedDescription)")
+        contentHandler(bestAttemptContent)
+      }
+    }


Again, this switch/case indentation looks odd to me.

See: #2156 (comment)

chrisbobbe · 2026-02-27T07:34:25Z

+  final Map<Object?, Object?> payload;
+}
+
+class MutatedNotificationContent {


I wonder if "mutated" is the name we want here, or if there's something more specific we want to emphasize? The goal is to make "better"/"improved" notification content, right, not just different/changed content. 😛 #1265 says "improve the wording and formatting of notifications".

I had used MutatedNotificationContent name to correspond to UNMutableNotificationContent. Changed to "ImprovedNotificationContent".

chrisbobbe

And some more comments, including ones on the last commit, which I forgot to read last time around (oops).

chrisbobbe · 2026-02-28T02:08:21Z

  Future<void> toggleWakelock({required bool enable});
+
+  /// The iOS App Group identifier specified in the Xcode config.
+  static const iosAppGroupIdentifier = 'group.zulip.test';


This will eventually not say "test", right? Do we need a comment on the consequences of changing this in future (e.g. data loss?)

chrisbobbe · 2026-02-28T02:19:44Z

+
+  IosNotifFlutterApi iosNotifFlutterApi() => TestZulipBinding.instance.iosNotifFlutterApi!;
+
+  test('smoke', () async {


This needs a addTearDown(testBinding.reset);, I think.

chrisbobbe · 2026-02-28T02:22:40Z

+  IosNotifFlutterApi iosNotifFlutterApi() => TestZulipBinding.instance.iosNotifFlutterApi!;
+
+  test('smoke', () async {
+    IosNotificationService.init();


IosNotificationService has mutable state (isExecutingInExtension) which we don't want to leak between tests; probably it should have a reset method that tests call on teardown?

chrisbobbe · 2026-02-28T02:28:05Z


  /// The file path to use for the app database.
  static Future<File> _dbFile() async {
+    if (defaultTargetPlatform == TargetPlatform.iOS) return _maybeCopyIosDbFile();


Below this new early return, the comment starting with "What directory should we use?" now looks confusing:

It's a long comment, so it seems like the question it's asking doesn't have a simple answer.

…But the question has apparently been answered anyway, at least for iOS.

The answer still includes "on iOS" cases.

chrisbobbe · 2026-02-28T02:47:57Z

+    if (IosNotificationService.isExecutingInExtension) {
+      // In iOS Notification Service Extension we can't copy the file, as the
+      // extension will not have access to main app target's filesystem.
+      // We also can't return the App Group container path here when running in
+      // the extension, because it will cause creation of new empty DB file,
+      // which then the main app target will prefer because of the code above.
+      // So, abort if we are running in iOS Notification Service Extension and
+      // the database is not already migrated to the App Group container.
+      throw Exception('The database file in iOS app group container doesn\'t '
+        'exists, and when running in iOS Notification Service Extension the '
+        'database file can\'t be migrated');
+    }


I think some of this comment and error message can be removed/simplified if we make the dartdoc more explicit about why the things it does are needed.

Can we make the dartdoc clear that

This is essentially a one-time migration that enables the extension to work. The extension won't work without it because [etc.].

The migration can only be run by the app, not the app extension, because [etc.].

If this runs in the app extension before the migration has been done, [etc.] happens.

This reminds me of the data migration from the legacy app, right? Though less complex, because we're just straightforwardly copying a file instead of restructuring data and so on. Still, are there details from that former migration that would be helpful to think about here? (Like how to organize the code?) Do we need to set a flag somewhere tracking whether the migration is complete, like we do for other migrations? Would some test coverage be feasible? Etc. It would be embarrassing to cause data loss for people by mishandling a migration somehow. 🙂

Added the required context to the dartdoc.

Do we need to set a flag somewhere tracking whether the migration is complete, like we do for other migrations?

I don't think this is needed because it is only a single operation (copying the file), and would only be executed from Runner target, so no race condition between Runner and NotificationService.

Would some test coverage be feasible

Not sure how best to unit test this type of file copy migrations, if we did mocks for file system api that it would not really be testing actual code.

It would be embarrassing to cause data loss for people by mishandling a migration somehow.

Agreed.

chrisbobbe · 2026-02-28T02:53:51Z

+                      'to iOS app group container path: $containerDbFile'));
+      await nonContainerDbFile.copy(containerDbFile.path);
+
+      // TODO do we want to delete nonContainerDbFile here?


This seems like the kind of detail that we'd want to have a plan for, even if we don't do it right now, before merging this migration.

gnprice

Thanks! Here's a handful of comments from an initial skim. (I realize this hasn't been marked as fully revised for Chris's comments above.)

gnprice · 2026-03-04T23:43:52Z

@@ -0,0 +1,239 @@
+// Autogenerated from Pigeon (v26.1.7), do not edit directly.


This new generated file should be added to .gitattributes so that it gets collapsed in git diff.

(Better: a pattern that covers it and similar files.)

gnprice · 2026-03-04T23:46:13Z

+    // TODO rename the group identifier.
+    const iosAppGroupIdentifier = 'group.zulip.test';


Maybe group.zulip.app? Seems like this group is basically "the Zulip app, as a whole".

gnprice · 2026-03-04T23:52:05Z

+    if (defaultTargetPlatform == TargetPlatform.iOS) {
+      file = await _maybeCopyIosDbFile();
+    } else {
+      file = await _dbFile();


This complication seems like something that's not part of this caller's job to worry about. It should go inside the method this code is calling, so either _dbFile or some new method.

gnprice · 2026-03-04T23:55:19Z

+        'database file can\'t be migrated');
+    }
+
+    final nonContainerDbFile = await _dbFile();


Echoing and expanding a bit on what Chris said at #2156 (comment) : definitely take the code in _dbFile as open to revision with the changes you're making here. The filename that the existing logic there produces is no longer really "the DB file" for the app on iOS; so either the method's name, or its logic, or both, should change. Likely the comment in it should then change too. The goal is for the new code after your changes to have a logic and structure that's clear to the reader, even without them having seen what the code looked like previously.

rajveermalviya · 2026-03-05T15:50:05Z

Thanks for the reviews @chrisbobbe, @gnprice! Pushed a new revision, PTAL.

chrisbobbe · 2026-03-06T01:38:04Z

Would you try the following:

In step 4 of https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter we should add the script pre-action to the "Runner" scheme (the scheme we always use) instead of the auto-generated "NotificationService" scheme.
Remove the workaround script phase that we added to the "NotificationService" target's build phases.

Possibly that step 4 should be changed. It seems odd to add the pre-action only on the scheme dedicated to building the app extension in isolation. In our case, at least, I think we can just delete that (the auto-generated "NotificationService" scheme)… Our default scheme, "Runner", which has become a confusing name, builds both the "Runner" target and the "NotificationService" target, which is probably always what we want.

The goal is to avoid running the heavier xcode_backend.sh build command (which runs flutter assemble) twice. This should cause the lightweight xcode_backend.sh prepare command to run once, instead; that will just copy the Flutter framework to a place where Xcode will find it when building the NotificationService target, meeting the need discussed at flutter/flutter#103344 (comment).

See flutter/flutter#146256 (which is basically the same solution to this problem as it appears in a different context):

In addition, a "Pre-action" script will be added via altering of the Runner.xcscheme. This script will invoke the flutter tool to copy the Flutter/FlutterMacOS framework to the BUILT_PRODUCTS_DIR directory before the build starts. This is needed because plugins need to be linked to the Flutter framework and fortunately Swift Package Manager automatically uses BUILT_PRODUCTS_DIR as a framework search path.

chrisbobbe · 2026-03-06T02:29:52Z

If that works, then try reverting step 3 of https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter :

In Xcode, disable user script sandboxing

That seems to be a security feature that applies to script build phases: you limit the script's permissions to specific input and output files. But the instructions don't ask you to add a script build phase, and if my suggestion above worked, you won't have added any.

I don't think there's a version of this feature that applies to a pre-action script. In the UI to configure a pre-action script, there's no way to specify input/output files.

chrisbobbe · 2026-03-06T03:12:08Z

For step 5 of https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter:

In Xcode, share the build configurations.

would you make a separate commit for that (or maybe squash it into the commit that creates the new target)? It's an interesting change because:

it can help us simplify our min-iOS-version config (I'll explain in a separate comment)
it pulls in some build config specified by Flutter, including some CocoaPods-related stuff; see for example ios/Flutter/Release.xcconfig :

#include? "Pods/Target Support Files/Pods-Runner/Pods-Runner.release.xcconfig"
#include "Generated.xcconfig"
#include "Zulip.xcconfig"

Probably the app extension's Flutter behavior depends on some of that config. But we might find out later that these defaults aren't a perfect fit for app extensions; it won't be top-of-mind when Flutter makes changes to it.

chrisbobbe · 2026-03-06T03:23:18Z

ios/Flutter/Debug.xcconfig and ios/Flutter/Release.xcconfig both import ios/Flutter/Zulip.xcconfig. We should be able to set IPHONEOS_DEPLOYMENT_TARGET with one line in Zulip.xcconfig and have it flow to all of our targets, as long as all of the targets share ios/Flutter/Debug.xcconfig and ios/Flutter/Release.xcconfig. This is what you ensured for the NotificationService target in that step 5 "share the build configurations" that I mentioned in my previous comment. Then we should be able to delete all references to IPHONEOS_DEPLOYMENT_TARGET in project.pbxproj and delete all the instructions at the top of ios/Podfile about navigating the Xcode GUI. 🎉

This moves IPHONEOS_DEPLOYMENT_TARGET config to Zulip.xcconfig, instead of it being specified for different Xcode config sections. This vastly simplifies changing the iOS deployment target later, allowing us to change a single variable, instead of navigating through the Xcode UI to change for multiple targets (two currently; Runner and RunnerTests). See: zulip#2156 (comment)

This carries out the steps 3-4 from guide for "Add iOS app extension" section in the docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#add-extension Then ran 'swift-format' over the generated Swift code. (In the Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'.) Finally, this also carries out step 5 from "Open a Flutter app in an iOS app extension" section in the docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter which shares the build configurations between extension target and Runner target. Allowing the extension target to: - Inherit the IPHONEOS_DEPLOYMENT_TARGET config value, from Zulip.xcconfig which {Debug,Release}.xcconfig imports. - Also it pulls in some build config specified by Flutter, including some CocoaPods-related stuff; see for example ios/Flutter/Release.xcconfig. See: zulip#2156 (comment)

…xtension Implement initial setup for headless FlutterEngine for running Dart code in NotificationService extension. This carries out the step 5 from guide for "Open a Flutter app in an iOS app extension" section in the docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter This carries out the steps 2-3 from guide for "Register plugins" section in the docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#register-plugins Additionally adds the following pre-action script for the Runner scheme: /bin/sh "$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh" prepare This script copies the Flutter framework to a place where Xcode will find it when building the NotificationService target. See discussion: zulip#2156 (comment)

This moves IPHONEOS_DEPLOYMENT_TARGET config to Zulip.xcconfig, instead of it being specified for different Xcode config sections. This vastly simplifies changing the iOS deployment target later, allowing us to change a single variable, instead of navigating through the Xcode UI to change for multiple targets (two currently; Runner and RunnerTests). See: zulip#2156 (comment)

Carries out the steps 3-4 from "Add iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#add-extension In step 3, the Flutter docs walk through adding a "Share Extension" target, we instead add a "Notification Service Extension" target, as described in Apple's documentation: https://developer.apple.com/documentation/usernotifications/modifying-content-in-newly-delivered-notifications#Add-a-service-app-extension-to-your-project The Swift file generated in step 3 was formatted using `swift-format` (In Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'). Also carries out step 5 from the "Open a Flutter app in an iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter this will share the build configurations between the extension target and the Runner target, allowing the extension target to: - Inherit the IPHONEOS_DEPLOYMENT_TARGET config value, from Zulip.xcconfig which {Debug,Release}.xcconfig imports. - Also it pulls in some build config specified by Flutter, including some CocoaPods-related stuff; see for example ios/Flutter/Release.xcconfig. See discussion: zulip#2156 (comment) Also made following other changes to Xcode build settings for the generated NotificationService target: - Deleted "iOS Deployment Target" config specified for target, so that when building the target, the inherited value from Zulip.xcconfig will be used. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Deployment, then pressed the Delete key on keyboard after selecting "iOS Deployment Target". - Updated the versioning information to match the Runner target. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Versioning, and changing the following values: * Changed "Current Project Version" to "$(FLUTTER_BUILD_NUMBER)" (in Xcode text field, typed without the double-quotes), where the generated value was "1". * Changed "Marketing Version" to "$(FLUTTER_BUILD_NAME)" (same as previous one, no double-quotes), where the default value was "1.0". * Changed "Versioning System" to "Apple Generic", where the default was set to "None". See discussion: zulip#2156 (comment)

…xtension Implement initial setup of a headless FlutterEngine for running Dart code in the NotificationService extension. This involves some Swift code to start the engine and drive its event loop, and a trivial Dart function to serve as the entry point. It also involves some build setup. For the build setup, this carries out the steps 2-3 from the "Register plugins" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#register-plugins and adds the following pre-action script for the Runner scheme: /bin/sh "$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh" prepare by following same steps listed under step 4 in "Open a Flutter app in an iOS app extension" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter but for the Runner scheme instead of the extension scheme. And also selected the Runner target instead of the extension target for the "Provide build settings" drop-down list. That script copies the Flutter framework to a place where Xcode will find it when building the NotificationService target. See discussion: zulip#2156 (comment) We are skipping the step 3 of "Open a Flutter app in an iOS app extension": https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter because disabling user script sandboxing is only needed if we had added a "Run Script" build phase without specifying input and output files. It seems it is unnecessary for the pre-action script because Xcode UI for adding the script doesn't have the options for specifying input and output files. See discussion: zulip#2156 (comment)

rajveermalviya · 2026-04-09T14:50:18Z

Thanks for the review @gnprice! Pushed a new revision, including a new commit which removes the simlated push notification docs, PTAL.

gnprice

Thanks for the revision! Just small comments below, plus what I said in the chat thread at #mobile-team > managing iOS notifications @ 💬:

Please write these instructions down in a file in docs/ and include that in the PR branch. It's important that people be able to do development on this code.

I've also now successfully followed those instructions and demonstrated this PR working 🎉:

On a physical iPhone, with flutter run --release, with an edit to the ImprovedNotificationContent constructor call so as to alter the notification title, the notifications show up with that alteration.
In the simulator, with plain flutter run so a debug build, the same observation plus there are log messages in Console.app from the new Dart code.

gnprice · 2026-04-09T22:46:28Z

+    return true;
+  }());
+
+  assert(debugLog('dart: iosNotificationServiceMain'));


Are these "dart:" prefixes needed? When I run this, the lines I see in Console.app look like "flutter: dart: iosNotificationServiceMain"; seems like "flutter:" is already signalling where this comes from.

(And in general we don't put that kind of prefix into our log messages.)

gnprice · 2026-04-09T22:50:04Z

+    // Adapted from: https://github.com/flutter/flutter/blob/65b1ec407/engine/src/flutter/fml/platform/darwin/message_loop_darwin.mm#L44-L62
+    let kDistantFuture = 1.0e10
+    while loopRunning {
+      let result = CFRunLoopRunInMode(.defaultMode, kDistantFuture, true)
+      if result == .stopped || result == .finished {
+        loopRunning = false
+      }
+    }


Cool. Writing down here what I said on our call today: I think the key thing I'd want here, then, is just to make these two versions look as similar as possible in the ways that don't matter, so that the change that does matter stands out better.

The switch/case seems like a good idea, since it helps make visible what the different result values are here.

gnprice · 2026-04-09T22:52:38Z

+  /// Wraps [IosNotifFlutterApi.setUp], provided an implementation of
+  /// [IosNotifFlutterApi] whose methods will be called from native code
+  /// (Swift in this case).
+  void setupIosNotifFlutterApi(IosNotifFlutterApi api);


The part about how IosNotifFlutterApi will get called seems like it's really a fact about that class.

Hmm in fact: that class should get some dartdoc. 🙂 (You can write it in the Pigeon file, and it'll get copied to the generated class.)

gnprice · 2026-04-09T22:53:23Z

+@FlutterApi()
+abstract class IosNotifFlutterApi {
+  @async
+  ImprovedNotificationContent didReceivePushNotification(NotificationContent content);


This class and method should each get some dartdoc. For examples, see pigeon/android_notifications.dart.

gnprice · 2026-04-09T22:53:37Z

+  final Map<Object?, Object?> payload;
+}
+
+class ImprovedNotificationContent {


Similarly these two data classes.

gnprice · 2026-04-09T22:54:14Z

+    final title = alertData['title'] as String;
+    final body = alertData['body'] as String?;
+
+    return ImprovedNotificationContent(title: title, body: body);


bump #2156 (comment)

gnprice · 2026-04-09T22:58:09Z

    (_notificationPigeonApi ??= FakeNotificationPigeonApi());
  FakeNotificationPigeonApi? _notificationPigeonApi;

+  IosNotifFlutterApi? get iosNotifFlutterApi => _iosNotifFlutterApi;


This is part of the API that test code is meant to use, so it should get some dartdoc (just like the other members that TestZulipBinding introduces as new API).

gnprice · 2026-04-09T22:58:59Z

    (_notificationPigeonApi ??= FakeNotificationPigeonApi());
  FakeNotificationPigeonApi? _notificationPigeonApi;

+  IosNotifFlutterApi? get iosNotifFlutterApi => _iosNotifFlutterApi;


nit: Basically all callers of this getter are going to want to assume this has been set, right? So we can simplify that for them:

Suggested change

IosNotifFlutterApi? get iosNotifFlutterApi => _iosNotifFlutterApi;

IosNotifFlutterApi get iosNotifFlutterApi => _iosNotifFlutterApi!;

gnprice · 2026-04-09T23:06:46Z

+abstract class IosNotificationService {
+  const IosNotificationService._();


Stray constructor. Looks like a mismerge — it pops up in the main commit, after the class appears. Here's from git range-diff:

-@visibleForTesting - class IosNotificationService { - const IosNotificationService._(); - + abstract class IosNotificationService { ++ const IosNotificationService._(); ++ + /// Whether the currently executing context is + /// iOS notification service app extension. + static bool isExecutingInExtension = false;

(It's clearer with the full coloration:

)

This moves IPHONEOS_DEPLOYMENT_TARGET config to Zulip.xcconfig, instead of it being specified for different Xcode config sections. This vastly simplifies changing the iOS deployment target later, allowing us to change a single variable, instead of navigating through the Xcode UI to change for multiple targets (two currently; Runner and RunnerTests). See: zulip#2156 (comment)

Carries out the steps 3-4 from "Add iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#add-extension In step 3, the Flutter docs walk through adding a "Share Extension" target, we instead add a "Notification Service Extension" target, as described in Apple's documentation: https://developer.apple.com/documentation/usernotifications/modifying-content-in-newly-delivered-notifications#Add-a-service-app-extension-to-your-project The Swift file generated in step 3 was formatted using `swift-format` (In Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'). Also carries out step 5 from the "Open a Flutter app in an iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter this will share the build configurations between the extension target and the Runner target, allowing the extension target to: - Inherit the IPHONEOS_DEPLOYMENT_TARGET config value, from Zulip.xcconfig which {Debug,Release}.xcconfig imports. - Also it pulls in some build config specified by Flutter, including some CocoaPods-related stuff; see for example ios/Flutter/Release.xcconfig. See discussion: zulip#2156 (comment) Also made following other changes to Xcode build settings for the generated NotificationService target: - Deleted "iOS Deployment Target" config specified for target, so that when building the target, the inherited value from Zulip.xcconfig will be used. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Deployment, then pressed the Delete key on keyboard after selecting "iOS Deployment Target". - Updated the versioning information to match the Runner target. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Versioning, and changing the following values: * Changed "Current Project Version" to "$(FLUTTER_BUILD_NUMBER)" (in Xcode text field, typed without the double-quotes), where the generated value was "1". * Changed "Marketing Version" to "$(FLUTTER_BUILD_NAME)" (same as previous one, no double-quotes), where the default value was "1.0". * Changed "Versioning System" to "Apple Generic", where the default was set to "None". See discussion: zulip#2156 (comment)

…xtension Implement initial setup of a headless FlutterEngine for running Dart code in the NotificationService extension. This involves some Swift code to start the engine and drive its event loop, and a trivial Dart function to serve as the entry point. It also involves some build setup. For the build setup, this carries out the steps 2-3 from the "Register plugins" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#register-plugins and adds the following pre-action script for the Runner scheme: /bin/sh "$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh" prepare by following same steps listed under step 4 in "Open a Flutter app in an iOS app extension" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter but for the Runner scheme instead of the extension scheme. And also selected the Runner target instead of the extension target for the "Provide build settings" drop-down list. That script copies the Flutter framework to a place where Xcode will find it when building the NotificationService target. See discussion: zulip#2156 (comment) We are skipping the step 3 of "Open a Flutter app in an iOS app extension": https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter because disabling user script sandboxing is only needed if we had added a "Run Script" build phase without specifying input and output files. It seems it is unnecessary for the pre-action script because Xcode UI for adding the script doesn't have the options for specifying input and output files. See discussion: zulip#2156 (comment)

rajveermalviya · 2026-04-10T17:15:15Z

Thanks for the review @gnprice! Pushed a new revison, PTAL.

gnprice

Thanks! Comments below.

gnprice · 2026-04-10T20:13:40Z

+      case .timedOut:
+        // Ideally this should be unreachable because the time out is much
+        // larger than the 30 second limit for this function. But continue
+        // looping here, matching the upstream implementation.
+        continue


The timeout is 1e10 seconds. That's over 300 years. 🙂 We don't need to try to reason about things like a 30-second limit for notification handling to conclude that this shouldn't ever happen.

gnprice · 2026-04-11T02:31:51Z

+      case .failure(let error):
+        contentHandler(bestAttemptContent)  // TODO(log)


Xcode gives me a warning here:

Immutable value 'error' was never used; consider replacing with '_' or removing it

gnprice · 2026-04-11T02:40:29Z

+      @unknown default:
+        fatalError()


So if I delete this case, the warning I get reads:

Switch covers known cases, but 'CFRunLoopRunResult' may have additional unknown values, possibly added in future versions; this is an error in the Swift 6 language mode

If a future iOS version adds some new value for this enum, do we really want a fatal error here? That doesn't seem right.

Instead let's do a // TODO(log) and either continue the loop or break out of it, not sure which.

gnprice · 2026-04-11T02:41:26Z

+    // TODO let FlutterEngine itself handle this, or expose an API that makes
+    //   this easier, maybe with something like:
+    //     https://github.com/flutter/flutter/pull/181645


This is calling for something that would be a change upstream, right? Let's use our little notation for indicating that:

Suggested change

// TODO let FlutterEngine itself handle this, or expose an API that makes

// this easier, maybe with something like:

// https://github.com/flutter/flutter/pull/181645

// TODO(upstream) let FlutterEngine itself handle this, or expose an API that makes

// this easier, maybe with something like:

// https://github.com/flutter/flutter/pull/181645

gnprice · 2026-04-11T02:44:12Z

+  /// The new title to use for the notification.
+  final String title;
+
+  /// The new body to use for the notification.
+  final String? body;


What's the meaning of body having a value of null? What's the motivation for body being nullable but title not?

In the previous revision, the reason body was nullable was because the E2EE payload only set the title key (to the value "New notification") (See docs). But I realize now that it is a detail that is not needed for this PR, because legacy payloads have both of these keys and that is what this PR demo-ing with anyway.

It will also not be needed for the next PR (#2230), because we would override the existing title and body anyway.

Also, UNMutableNotificationContent.body doesn't allow setting it to null. (I guess the API opts for empty strings to indicate absence?)

Anyways made the body non-nullable.

gnprice · 2026-04-11T02:46:52Z

+    bestAttemptContent =
+      (request.content.mutableCopy() as? UNMutableNotificationContent)
+
+    guard let bestAttemptContent = bestAttemptContent else {
+      contentHandler(request.content)  // TODO(log)
+      return
+    }


Can mutableCopy actually return nil? If not, we can simplify this logic.

(Pushed a revision with the space between both statements removed, as discussed on call.)

Cool. Writing down here the answer you gave on the call to my question above:

This guard isn't (just?) for mutableCopy returning nil, but for it returning an object not of type UNMutableNotificationContent. In that case, the as? would return nil.

That still seems probably impossible, though the docs aren't real clear about that. (The return type of mutableCopy is just Any; it's inherited from a more general interface, and that interface isn't very crisply typed.)

But this check is there in the template code that Xcode supplies, seen in an earlier commit in this PR. So OK, we'll follow that lead and keep the check.

gnprice · 2026-04-13T06:56:46Z

+    // Currently the non-E2EE APNs payloads do not trigger the
+    // NotificationService app extension, because they do not contain the
+    // required key-pair ('mutable-content': 1). Meaning this code is never
+    // executed by an unmodified Zulip Server, currently, it only exists as
+    // a scaffold for implementing support for E2EE notifications (#1764).
+    // That is why we are passing the title and body as-is here, to serve as
+    // a demo that #1265 is complete.
+    return ImprovedNotificationContent(title: title, body: body);


The logic of this explanation doesn't totally make sense to me; I'll try revising it at merge time.

Edited like so:

// This doesn't ultimately have any effect: it returns the same // title and body that the notification already had, so nothing changes. // Moreover this code doesn't run in the first place when talking to a // normal Zulip server, because the non-E2EE APNs payloads // don't contain the flag `'mutable-content': 1` and so // don't trigger the NotificationService app extension. // // The purpose of this code is to be a checkpoint on the way to supporting // E2EE notifications (#1764) and more generally client-side control over // notification behavior (#1265). It can be manually tested with // a server-side edit to set the `mutable-content` flag, plus other steps: // https://github.com/zulip/zulip-flutter/pull/2156#pullrequestreview-4085925962

This moves IPHONEOS_DEPLOYMENT_TARGET config to Zulip.xcconfig, instead of it being specified for different Xcode config sections. This vastly simplifies changing the iOS deployment target later, allowing us to change a single variable, instead of navigating through the Xcode UI to change for multiple targets (two currently; Runner and RunnerTests). See: zulip#2156 (comment)

Similar to previous commit, this moves DEVELOPMENT_TEAM config to Zulip.xcconfig, instead of it being specified for different Xcode config sections.

In the Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'.

Carries out the steps 3-4 from "Add iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#add-extension In step 3, the Flutter docs walk through adding a "Share Extension" target, we instead add a "Notification Service Extension" target, as described in Apple's documentation: https://developer.apple.com/documentation/usernotifications/modifying-content-in-newly-delivered-notifications#Add-a-service-app-extension-to-your-project The Swift file generated in step 3 was formatted using `swift-format` (In Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'). Also carries out step 5 from the "Open a Flutter app in an iOS app extension" section in the Flutter docs: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter this will share the build configurations between the extension target and the Runner target, allowing the extension target to: - Inherit the IPHONEOS_DEPLOYMENT_TARGET config value, from Zulip.xcconfig which {Debug,Release}.xcconfig imports. - Also it pulls in some build config specified by Flutter, including some CocoaPods-related stuff; see for example ios/Flutter/Release.xcconfig. See discussion: zulip#2156 (comment) Also made following other changes to Xcode build settings for the generated NotificationService target: - Deleted "iOS Deployment Target" config specified for target, so that when building the target, the inherited value from Zulip.xcconfig will be used. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Deployment, then pressed the Delete key on keyboard after selecting "iOS Deployment Target". - Updated the versioning information to match the Runner target. By navigating to project navigator (View > Navigators > Project) > NotificationService under TARGETS > Build Settings (tab) > Versioning, and changing the following values: * Changed "Current Project Version" to "$(FLUTTER_BUILD_NUMBER)" (in Xcode text field, typed without the double-quotes), where the generated value was "1". * Changed "Marketing Version" to "$(FLUTTER_BUILD_NAME)" (same as previous one, no double-quotes), where the default value was "1.0". * Changed "Versioning System" to "Apple Generic", where the default was set to "None". See discussion: zulip#2156 (comment)

…xtension Implement initial setup of a headless FlutterEngine for running Dart code in the NotificationService extension. This involves some Swift code to start the engine and drive its event loop, and a trivial Dart function to serve as the entry point. It also involves some build setup. For the build setup, this carries out the steps 2-3 from the "Register plugins" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#register-plugins and adds the following pre-action script for the Runner scheme: /bin/sh "$FLUTTER_ROOT/packages/flutter_tools/bin/xcode_backend.sh" prepare by following same steps listed under step 4 in "Open a Flutter app in an iOS app extension" section in Flutter's guide to iOS app extensions: https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter but for the Runner scheme instead of the extension scheme. And also selected the Runner target instead of the extension target for the "Provide build settings" drop-down list. That script copies the Flutter framework to a place where Xcode will find it when building the NotificationService target. See discussion: zulip#2156 (comment) We are skipping the step 3 of "Open a Flutter app in an iOS app extension": https://docs.flutter.dev/platform-integration/ios/app-extensions#creating-app-extension-uis-with-flutter because disabling user script sandboxing is only needed if we had added a "Run Script" build phase without specifying input and output files. It seems it is unnecessary for the pre-action script because Xcode UI for adding the script doesn't have the options for specifying input and output files. See discussion: zulip#2156 (comment)

rajveermalviya · 2026-04-14T18:48:49Z

Thanks for the review @gnprice! Pushed an update, PTAL.

Fixes zulip#1265. This uses Pigeons's handy `@FlutterApi` API to define an interface method that allows passing arguments from Swift to Dart and also get a return value back from Dart to Swift.

…roup This is required to share any files like database between both app (Runner) and the service extension targets.

This will allow the Dart code executing in the notification service extension to access the same database file as the main app target.

…ulator Our notification code on iOS now relies on using notification service app extension to be able to modify incoming push notifications. These instructions walk through how to trigger a simulated (fake) push notification on iOS Simulator, and this simulated (fake) notification do not trigger the notification service app extension. See: https://stackoverflow.com/a/79707485 Meaning these instructions are not useful anymore, so remove them.

gnprice · 2026-04-14T22:25:38Z

Thanks @rajveermalviya for all your work on this! Merging, with a comment edited as described at #2156 (comment) .

rajveermalviya force-pushed the pr-ios-notif-extension branch 4 times, most recently from dc78826 to 7b54648 Compare February 25, 2026 17:31

rajveermalviya marked this pull request as ready for review February 25, 2026 17:37

rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Feb 25, 2026

rajveermalviya assigned chrisbobbe Feb 25, 2026

rajveermalviya requested a review from chrisbobbe February 25, 2026 17:38

chrisbobbe reviewed Feb 27, 2026

View reviewed changes

chrisbobbe reviewed Feb 28, 2026

View reviewed changes

rajveermalviya force-pushed the pr-ios-notif-extension branch 2 times, most recently from 09ffdb5 to 3fbbe3b Compare March 3, 2026 15:40

gnprice mentioned this pull request Mar 4, 2026

[iOS] Share to Zulip from other apps #54

Open

gnprice reviewed Mar 4, 2026

View reviewed changes

rajveermalviya force-pushed the pr-ios-notif-extension branch 3 times, most recently from 948d69c to 8e328fb Compare March 5, 2026 15:26

rajveermalviya requested a review from chrisbobbe March 5, 2026 15:51

chrisbobbe mentioned this pull request Mar 5, 2026

store ios: Exclude the database file from iOS backup #2195

Merged

rajveermalviya force-pushed the pr-ios-notif-extension branch from 8e328fb to 3bb22d2 Compare March 10, 2026 10:24

rajveermalviya force-pushed the pr-ios-notif-extension branch from 01a3b25 to d463532 Compare April 9, 2026 14:36

rajveermalviya force-pushed the pr-ios-notif-extension branch from d463532 to fbe9e8d Compare April 9, 2026 14:36

rajveermalviya requested a review from gnprice April 9, 2026 14:50

gnprice reviewed Apr 9, 2026

View reviewed changes

rajveermalviya force-pushed the pr-ios-notif-extension branch from 2fe7b97 to 469d106 Compare April 10, 2026 11:32

rajveermalviya force-pushed the pr-ios-notif-extension branch from 469d106 to 768da83 Compare April 10, 2026 17:14

rajveermalviya requested a review from gnprice April 10, 2026 17:15

gnprice reviewed Apr 13, 2026

View reviewed changes

rajveermalviya added 4 commits April 14, 2026 10:56

ios [nfc]: Extract setting DEVELOPMENT_TEAM to Zulip.xcconfig

ad4c602

Similar to previous commit, this moves DEVELOPMENT_TEAM config to Zulip.xcconfig, instead of it being specified for different Xcode config sections.

ios [nfc]: In Xcode, run swift-format

7089d1a

In the Xcode menu bar, Editor -> Structure -> Format file with 'swift-format'.

rajveermalviya force-pushed the pr-ios-notif-extension branch from 768da83 to dc66f9f Compare April 14, 2026 10:32

rajveermalviya added 4 commits April 14, 2026 15:20

ios: Implement ability to intercept APNs push notifications from Dart

e482b0f

Fixes zulip#1265. This uses Pigeons's handy `@FlutterApi` API to define an interface method that allows passing arguments from Swift to Dart and also get a return value back from Dart to Swift.

ios: Add both Runner and NotificationService target to the same app g…

ed3fe5a

…roup This is required to share any files like database between both app (Runner) and the service extension targets.

store ios: Migrate database file to iOS App Group container path

5f6bdb5

This will allow the Dart code executing in the notification service extension to access the same database file as the main app target.


		IosNotifFlutterApi iosNotifFlutterApi() => TestZulipBinding.instance.iosNotifFlutterApi!;

		test('smoke', () async {

		@@ -0,0 +1,239 @@
		// Autogenerated from Pigeon (v26.1.7), do not edit directly.

		// TODO rename the group identifier.
		const iosAppGroupIdentifier = 'group.zulip.test';

	IosNotifFlutterApi? get iosNotifFlutterApi => _iosNotifFlutterApi;
	IosNotifFlutterApi get iosNotifFlutterApi => _iosNotifFlutterApi!;

		abstract class IosNotificationService {
		const IosNotificationService._();

		case .failure(let error):
		contentHandler(bestAttemptContent) // TODO(log)

Conversation

rajveermalviya commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajveermalviya commented Feb 25, 2026

Uh oh!

rajveermalviya commented Feb 26, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Mar 5, 2026

Uh oh!

chrisbobbe commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Mar 6, 2026

Uh oh!

rajveermalviya commented Feb 18, 2026 •

edited

Loading

rajveermalviya Mar 5, 2026 •

edited

Loading

chrisbobbe left a comment •

edited

Loading

chrisbobbe commented Mar 6, 2026 •

edited

Loading

chrisbobbe commented Mar 6, 2026 •

edited

Loading